Skip to content

Update OCaml references in transaction_partially_applied.rs #1584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: dw/coinbase-tx-doc
Choose a base branch
from
Open

Update OCaml references in transaction_partially_applied.rs #1584

wants to merge 1 commit into from

Conversation

@dannywillems
Copy link
Member

@dannywillems dannywillems commented Oct 31, 2025 •
edited
Loading

Summary

Updates OCaml references from URL format to the new structured format with
commit hash and verification date. Ported from mina-rust PR #1553.

Changes

  • FailureCollection impl: Updated OCaml reference format
  • apply_coinbase function: Updated OCaml reference and expanded
    documentation with comprehensive parameter descriptions, return values,
    error conditions, tests, and protocol documentation link
  • sub_account_creation_fee function: Updated OCaml reference format

@dannywillems dannywillems changed the base branch from develop to dw/coinbase-tx-doc October 31, 2025 14:52
@dannywillems
Update OCaml references in transaction_partially_applied.rs
bc5b5d8 
Updates OCaml references from URL format to the new structured format
with commit hash and verification date for:
- FailureCollection impl
- apply_coinbase function (with expanded documentation)
- sub_account_creation_fee function

Changes ported from mina-rust PR #1553.
@dannywillems dannywillems force-pushed the dw/update-links-caml-and-doc-tx-logic branch from 7235c53 to bc5b5d8 Compare October 31, 2025 14:53
@github-actions
Copy link

OCaml Reference Validation Results

Repository: https://github.com/MinaProtocol/mina.git
Branch: compatible
Status: ❌ Validation failed

Click to see full validation output 
Checking OCaml references against https://github.com/MinaProtocol/mina.git (branch: compatible)
Fetching current commit from compatible...
Current OCaml commit: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d

Validating references...
========================
✓ VALID: ledger/src/account/account.rs -> src/lib/mina_base/account.ml L:201-224
  ⚠ STALE COMMIT: fc6be4c58091c761f827c858229c2edf9519e941 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/for_tests.rs -> src/lib/transaction_logic/mina_transaction_logic.ml L:2285-2285
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/for_tests.rs -> src/lib/transaction_logic/mina_transaction_logic.ml L:2351-2356
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/for_tests.rs -> src/lib/transaction_logic/mina_transaction_logic.ml L:2407
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/mina_base/transaction_status.ml L:9-51
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/mina_base/transaction_status.ml L:452-454
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/mina_base/with_status.ml L:6-10
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/mina_base/fee_transfer.ml L:76-80
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/mina_base/fee_transfer.ml L:68-69
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/mina_base/coinbase.ml L:17-21
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/mod.rs -> src/lib/transaction/transaction.ml L:8-11
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/signed_command.rs -> src/lib/mina_base/signed_command_payload.ml L:34-48
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/signed_command.rs -> src/lib/mina_base/stake_delegation.ml L:11-13
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/signed_command.rs -> src/lib/mina_base/signed_command_payload.ml L:179-181
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/signed_command.rs -> src/lib/mina_base/signed_command_payload.ml L:239-243
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
✓ VALID: ledger/src/scan_state/transaction_logic/signed_command.rs -> src/lib/mina_base/signed_command_payload.ml L:352-362
  ⚠ STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
❌ INVALID: ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs
   Code at L:2197-2210 differs between commit bfd1009abdbee78979ff0343cc73a3480e862f58 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/bfd1009abdbee78979ff0343cc73a3480e862f58/src/lib/transaction_logic/mina_transaction_logic.ml#L2197-L2210
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2197-L2210
✓ VALID: ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs -> src/lib/transaction_logic/mina_transaction_logic.ml L:2074-2178
  ⚠ STALE COMMIT: 0063f0196d046d9d2fc8af0cea76ff30f51b49b7 (current: e8e5f9e0dd346abb4f993d614c73da0b87f91b3d)
❌ INVALID: ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs
   Code at L:607 differs between commit 2ee6e004ba8c6a0541056076aab22ea162f7eb3a and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/2ee6e004ba8c6a0541056076aab22ea162f7eb3a/src/lib/transaction_logic/mina_transaction_logic.ml#L607-L607
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L607-L607

Summary
=======
Total references found: 19
Valid references: 17
Invalid references: 2
Stale commits: 17

❌ Validation failed: 2 invalid reference(s) found

@dannywillems dannywillems mentioned this pull request Oct 31, 2025
Open
@dannywillems dannywillems force-pushed the dw/update-links-caml-and-doc-tx-logic branch from 44b5998 to bc5b5d8 Compare October 31, 2025 15:18
@github-actions
Copy link

⚠️ Code Reference Verification Failed

The documentation contains code references that do not match the current state of the codebase on the develop branch.

Issues Found

  • website/docs/developers/transactions/coinbase.md:144 - Code reference to ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs#L559-L561 differs from GitHub (develop branch). The referenced code may have been modified locally but not yet merged to develop.

Action Required

The code referenced in the documentation must be merged to develop before documentation can be added/modified.

Please follow this workflow:

  1. Merge the code changes to develop first (this PR or a separate code PR)
  2. Create a follow-up PR with the documentation updates that reference the merged code
  3. The verification will pass once the code is available on develop

See the documentation guidelines for more information about the two-PR workflow.

richardpringle
richardpringle requested changes Oct 31, 2025
View reviewed changes
Copy link

@richardpringle richardpringle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the permalinks

ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs
}

/// <https://github.com/MinaProtocol/mina/blob/bfd1009abdbee78979ff0343cc73a3480e862f58/src/lib/transaction_logic/mina_transaction_logic.ml#L2197C1-L2210C53>
/// OCaml reference: src/lib/transaction_logic/mina_transaction_logic.ml L:2197-2210
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be a permalink; I think it should remain a permalink.

ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs
Comment on lines +402 to +403
/// Commit: bfd1009abdbee78979ff0343cc73a3480e862f58
/// Last verified: 2025-10-16
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why go through the trouble of adding this information. Do you plan on updating this comment every time you check the link? It's better to just have a timestamp on the commit and to use git-blame. One of the major benefits of the permalinks is that it's usually relatively easy to observe any changes from the last time this was the most relevant commit.

ledger/src/scan_state/transaction_logic/transaction_partially_applied.rs
/// `[[];[failure-of-coinbase]]`
/// Fee transfer fails and coinbase succeeds:
/// `[[failure-of-fee-transfer];[]]`
/// Applies a coinbase transaction to the ledger.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll admit pedantry here, but this is a grammatically incorrect sentence; there's no subject. If you want to write an incomplete sentence, all you have to do is remove the period.

Suggested change
/// Applies a coinbase transaction to the ledger.
/// Applies a coinbase transaction to the ledger

Haha... I won't be mad if you just dismiss this review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richardpringle richardpringle richardpringle requested changes

Assignees

No one assigned

Labels

no changelog

Projects

Rust node
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dannywillems @richardpringle